Skip to content

Fix: privacy request retry 500 when Redis cache expires and location is required#7729

Open
jjdaurora wants to merge 2 commits intomainfrom
jjdaurora/ENG-3066
Open

Fix: privacy request retry 500 when Redis cache expires and location is required#7729
jjdaurora wants to merge 2 commits intomainfrom
jjdaurora/ENG-3066

Conversation

@jjdaurora
Copy link
Contributor

@jjdaurora jjdaurora commented Mar 23, 2026 •
edited
Loading

Ticket ENG-3066

Description Of Changes

When a privacy request's Redis identity cache expires, the retry endpoint routes through resubmit_privacy_request, which rebuilds the request from the DB. location was the only DB-persisted field not carried forward into the PrivacyRequestResubmit payload, causing _validate_required_location_fields to re-run against the live Privacy Center config and raise a PrivacyRequestError — even though location was provided at original submission time.
Fix passes location=existing_privacy_request.location in the PrivacyRequestResubmit constructor, consistent with how all other DB-persisted fields are already handled. This satisfies the early-return guard in _validate_required_location_fields and requires no schema changes since location is already an optional field on PrivacyRequestCreate.

Code Changes

  • privacy_request_service.py:562 — add location to PrivacyRequestResubmit constructor in resubmit_privacy_request

Steps to Confirm

Submit a privacy request with a Privacy Center config that has a LocationCustomPrivacyRequestField marked required: true, providing a location value
Manually expire the Redis identity cache keys: redis-cli --scan --pattern "id--identity-*" | xargs redis-cli del
Retry the request via POST /api/v1/privacy-request//retry — confirm it succeeds (previously 500)
Confirm a request retried with a warm cache is unaffected

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@jjdaurora jjdaurora requested a review from a team as a code owner March 23, 2026 15:33
@jjdaurora jjdaurora requested review from johnewart and removed request for a team March 23, 2026 15:33
@vercel
Copy link
Contributor

vercel bot commented Mar 23, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Actions Updated (UTC)
fides-plus-nightly Ignored Ignored Preview Mar 23, 2026 4:03pm
fides-privacy-center Ignored Ignored Mar 23, 2026 4:03pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 23, 2026

Greptile Summary

This PR fixes a targeted bug where retrying a privacy request would return a 500 error when the Redis identity cache had expired and the Privacy Center config required a location field. The root cause was that resubmit_privacy_request reconstructed the request from the database but omitted location from the PrivacyRequestResubmit payload — even though it is persisted in the DB — causing _validate_required_location_fields to re-run and raise a PrivacyRequestError.

  • The one-line fix adds location=existing_privacy_request.location to the PrivacyRequestResubmit constructor, consistent with how all other DB-persisted fields (source, property_id, consent_preferences, etc.) are already handled.
  • _validate_required_location_fields short-circuits when location is truthy (line 200), so passing the stored value satisfies the guard without any schema changes.
  • No regression test was added to verify that resubmit_privacy_request preserves location when a required LocationCustomPrivacyRequestField config is present; adding one would prevent future regressions.

Confidence Score: 5/5

  • This PR is safe to merge — the fix is minimal, correct, and consistent with existing patterns for all other persisted fields.
  • Single-line, low-risk fix that mirrors the exact pattern used for every other DB-persisted field in the same constructor. The location field is already Optional[str] on PrivacyRequestCreate, PrivacyRequestResubmit inherits it, and the validator short-circuits when the value is present. The only remaining gap is a missing regression test (P2), which does not block merge.
  • No files require special attention.

Important Files Changed

Filename Overview
src/fides/service/privacy_request/privacy_request_service.py Adds location=existing_privacy_request.location to the PrivacyRequestResubmit constructor in resubmit_privacy_request, ensuring the persisted location value is carried forward on retry when the Redis cache has expired.

Reviews (1): Last reviewed commit: "PrivacyRequestResubmit; location=existi..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

@johnewart johnewart Awaiting requested review from johnewart johnewart is a code owner automatically assigned from ethyca/backend

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jjdaurora